Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small improvements to searchspaces and simulation mode #251

Merged
merged 7 commits into from
May 23, 2024

Conversation

fjwillemsen
Copy link
Collaborator

@fjwillemsen fjwillemsen commented Apr 12, 2024

This pull request solves various small issues regarding search spaces and simulation mode:

  • The constraint parser now works correctly when round brackets surround a restriction.
  • Fixed an issue where the automatically added check that the product of block_sizes does not exceed max_threads was not added back to the restrictions, which were later rechecked for each configuration if check_restrictions is enabled, causing a discrepancy.
  • Constraints can now also be evaluated from a tuple combination by the check_restrictions function.
  • An empty tuple is added for unknown parameters of ignored restrictions.
  • The MinProd constraint was added to the conversion function for backwards compatibility.

Comment on lines 105 to 107
# "ngangs",
# "nworkers",
# "vlength",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these lines commented?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These block size names were added for OpenACC. The default block sizes were added automatically if the block size is not defined by the user, causing issues with these parameters outside of OpenACC. I spoke with @isazi about this, if I understood correctly these are no longer needed, hence commented pending removal.

Comment on lines 287 to 295
if isinstance(restrict, Constraint):
restrict = convert_constraint_restriction(restrict)
if not restrict(list(selected_params.values())):
valid = False
break
else:
if not restrict(**selected_params):
valid = False
break
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems our automated linting system 'SonarCloud' isn't too happy about this level of nesting control flow. Maybe we can break things up into smaller chunks?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've refactored this into two separate functions, SonarCloud is happy again.

@fjwillemsen fjwillemsen changed the title Solves various issues with searchspaces and simulation mode Small improvements to searchspaces and simulation mode Apr 28, 2024
Copy link

sonarcloud bot commented May 23, 2024

Quality Gate Passed Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@benvanwerkhoven benvanwerkhoven merged commit 0fc4ad2 into master May 23, 2024
3 checks passed
@benvanwerkhoven
Copy link
Collaborator

Thanks!

@fjwillemsen fjwillemsen deleted the simulation-searchspace-improvements branch May 23, 2024 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants